Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Define an inherited policy for feature in container at origin #501

Merged
merged 3 commits into from
Jan 9, 2023
Merged

Fix Define an inherited policy for feature in container at origin #501

merged 3 commits into from
Jan 9, 2023

Conversation

arichiv
Copy link
Member

@arichiv arichiv commented Dec 20, 2022

The text implies only the inherited policy of the document should
be checked, but even if the containing document has the feature
enabled the document could set a permissions policy that denied access
to the specific origin. We must check the actual policy for the
containing document to know. Chrome is already doing this.

We can accomplish this by adapting both 9.8.2 and 9.8.3 to
use Is feature enabled in document for origin? but have the
first check the parent origin and the second check the
actual origin.


Preview | Diff

The text implies only the inherited policy of the document should
be checked, but even if the containing document has the feature
enabled the document could set a permissions policy that denied access
to the specific origin. We must check the actual policy for the
containing document to know. Chrome is already doing this.
@arichiv arichiv closed this Dec 20, 2022
@arichiv arichiv reopened this Dec 20, 2022
@clelland
Copy link
Collaborator

clelland commented Jan 9, 2023

This looks good, and matches the intention, and, I think, the actual implementation in Chrome. Happy to merge this.

Note that this behaviour will change when #480 is fixed -- for that issue, we actually do want a way to block the use of a feature in the current frame, but retain the ability to delegate it to a subframe, but that will require parser changes, as well as support from this algorithm.

@clelland clelland merged commit 81dcf77 into w3c:main Jan 9, 2023
github-actions bot added a commit that referenced this pull request Jan 9, 2023
…501)

SHA: 81dcf77
Reason: push, by clelland

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 9, 2023
I'm fixing the spec to match what chrome does:
w3c/webappsec-permissions-policy#501
which is to check both the parent and current origin
against the allowlist as well as the inherited policy.

The comments here should be updated to reflect what the code actually
does the reduce confusion.

Bug: 1401055
Change-Id: I078da599b638202aeaf3ea4cbcc9003a911b9612
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4116819
Auto-Submit: Ari Chivukula <[email protected]>
Reviewed-by: Ian Clelland <[email protected]>
Commit-Queue: Ian Clelland <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1090570}
clelland added a commit that referenced this pull request Jan 11, 2023
#501 Updated the "Is feature enabled in document for origin" algorithm, which left the `policy` variable in that algorithm unused. This removes it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants